Customer Search: Implement UI #7880

iamgabrielma · 2022-10-19T09:03:13Z

Part of #7741
Branched from #7848 , that PR needs to be merged first.

Description

This PR implements the basic UI for searching Customers. The scope of this PR is limited to display a search view when we tap on the search icon, display Customer results matching our keyword, and pass the Customer data back to our EditOrderAddressForm view when we tap in any of the Customers. Filling the data to corresponding fields will be done in a future PR.

Changes

Adds a OrderCustomerListView, which is a SwiftUI wrapper for the existing SearchViewController.
Adds a CustomerSearchUICommand class, which is the implementation of the existing SearchUICommand for Customer search.

Testing instructions

Go to Orders > + > Create new Order > + Add Customer details > Tap on the magnifying glass icon:

Search for a Customer name, and tap on it:

Check if details are printed in the Xcode console. This will display steps 1 to 4, which represents the Customer data flow to EditOrderAddressForm, which we'll use to fill the Order with Customer details in a future PR.

1 - Customer tapped
2 - Customer ID: 5 - Name: Hello)
3 - Customer Callback. Fill Order data with Customer details
4 - Customer ID: 5 - Name: Hello)

wpmobilebot · 2022-10-19T09:25:38Z

You can test the changes from this Pull Request by:

Clicking here or scanning the QR code below to access App Center
Then installing the build number pr7880-da5e401 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

iamgabrielma · 2022-10-20T04:21:43Z

...erce/Classes/ViewRelated/Orders/Order Creation/CustomerSection/CustomerSearchUICommand.swift

+final class CustomerSearchUICommand: SearchUICommand {
+
+    typealias Model = Customer
+    typealias CellViewModel = TitleAndSubtitleAndStatusTableViewCell.ViewModel


Happy to get a different recommendation for the CellView we want to use here, at the moment I'm using TitleAndSubtitleAndStatusTableViewCell just for convenience, as must conform to SearchResultCell there are several options to choose from without having to create a new one (or I could create a new one as well!)

Given that you're going to have the avatar in there too, you might want to try adding SearchResultCell conformance to LeftImageTitleSubtitleTableViewCell?

The only thing it won't do is any attributed text in the subtitle for an underlined email as on Android... but I don't personally think that should be a blocker.

iamgabrielma · 2022-10-25T02:25:55Z

@rachelmcr @ThomazFB @Ecarrion please leave a comment if any of you plan to review the PR today, otherwise I'll review it with @joshheald during our pairing session. Thanks!

iamgabrielma · 2022-10-25T08:55:42Z

...erce/Classes/ViewRelated/Orders/Order Creation/CustomerSection/CustomerSearchUICommand.swift

+    }
+
+    func searchResultsPredicate(keyword: String) -> NSPredicate? {
+        return NSPredicate(format: "ANY searchResults.keyword = %@", keyword)


Needs to add siteID! We'll add it in a future PR/commit as this is currently behind a feature flag.

Yep! Don't forget both places, this is the one for the CustomerSearchResults rather than the Customer

Definitely, I went ahead and changed it in this PR as was a quick one: a8dd778

joshheald

Well done. I was able to test it and get some search results showing in the app.

The comments here can all be addressed in a future PR, nothing is blocking.

Other general things I noticed, outside the scope of this PR, for your notes:

There are lots of network requests to /wc/v3/customers/0, which will always fail, these could be filtered out.
There's no empty state yet, just a blank white screen.
The search doesn't behave the same as on Android. On Android, search looks at the email field only, and on iOS, it looks at the name fields only. Apologies if I'm forgetting some context here, if Android are going to change that...

joshheald · 2022-10-25T17:26:40Z

...erce/Classes/ViewRelated/Orders/Order Creation/CustomerSection/CustomerSearchUICommand.swift

+final class CustomerSearchUICommand: SearchUICommand {
+
+    typealias Model = Customer
+    typealias CellViewModel = TitleAndSubtitleAndStatusTableViewCell.ViewModel


Given that you're going to have the avatar in there too, you might want to try adding SearchResultCell conformance to LeftImageTitleSubtitleTableViewCell?

The only thing it won't do is any attributed text in the subtitle for an underlined email as on Android... but I don't personally think that should be a blocker.

joshheald · 2022-10-25T17:27:51Z

WooCommerce/WooCommerce.xcodeproj/project.pbxproj

 		4598298028574688003A9AFE /* Inject */ = {
 			isa = XCSwiftPackageProductDependency;
-			package = 4598297F28574688003A9AFE /* XCRemoteSwiftPackageReference "Inject.git" */;
+			package = 4598297F28574688003A9AFE /* XCRemoteSwiftPackageReference "Inject" */;


Has Inject changed for any particular reason? What happens if you revert these changes?

Good catch! And that's a good question:

The only difference I'm seeing is that when I revert the changes is that I can see the iOS 15.0 minimum support warnings for YosemiteTests related to kUTTypeJPEG deprecation.

This most likely comes from me switching to iOS15 temporarily when working on this, as at the same time we were upgrading the app minimum version for the app and I wanted to see if my changes came with potential deprecations. Then when I switched back to iOS14, I may have missed the changes in project.pbxproj.

I see these changes are part of trunk already, but I reverted here: da5e401 as this change shouldn't go with my PR anyway 👍

joshheald · 2022-10-25T17:31:16Z

...erce/Classes/ViewRelated/Orders/Order Creation/CustomerSection/CustomerSearchUICommand.swift

+    func createResultsController() -> ResultsController<StorageCustomer> {
+        let storageManager = ServiceLocator.storageManager
+        let descriptor = NSSortDescriptor(keyPath: \StorageCustomer.customerID, ascending: false)
+        return ResultsController<StorageCustomer>(storageManager: storageManager, sortedBy: [descriptor])


This will display StorageCustomers from Core Data for any site, not just the current one, which could lead to confusion for some merchants with multiple stores.

If you add a predicate specifying the siteID, that should resolve the issue. No need to block this PR since it's behind a feature flag.

Thanks! I committed the changes a8dd778 with this one as we went through these in our pairing as was pretty quick 👍

joshheald · 2022-10-25T17:37:35Z

...erce/Classes/ViewRelated/Orders/Order Creation/CustomerSection/CustomerSearchUICommand.swift

+    func createCellViewModel(model: Customer) -> TitleAndSubtitleAndStatusTableViewCell.ViewModel {
+        return CellViewModel(
+            id: "\(model.customerID)",
+            title: "\(model.firstName ?? "") \(model.lastName ?? "")",


This... might be the right thing to do given our data source, but names are complicated, and simply concatenating them isn't correct in all locales.

See PersonNameComponentsFormatter docs for more info. There's not necessarily anything to do here (as I don't know whether we have enough in our data source to correctly make a formatter) but I thought it would be useful for you to know about if you don't already.

Thanks for the link, I definitely didn't know about PersonNameComponentsFormatter. I agree with you, concatenating strings doesn't look good to me either and I'll look for cleaner alternatives (if possible). I'll dig into this document and make any necessary improvements on another PR.

joshheald · 2022-10-25T17:41:24Z

...erce/Classes/ViewRelated/Orders/Order Creation/CustomerSection/CustomerSearchUICommand.swift

+            id: "\(model.customerID)",
+            title: "\(model.firstName ?? "") \(model.lastName ?? "")",
+            subtitle: model.email,
+            accessibilityLabel: "",


Accessibility Label is required here... see what it does in VoiceOver without it: it just announces:

Button

Without any detail about the name of the user or anything else about the search result.

If you're moving to another cell, you may not have to do anything, usually we get VoiceOver in cells for free, but in this case we are specific about what the label should be in the view model.

joshheald · 2022-10-25T17:42:39Z

...erce/Classes/ViewRelated/Orders/Order Creation/CustomerSection/CustomerSearchUICommand.swift

+    }
+
+    func searchResultsPredicate(keyword: String) -> NSPredicate? {
+        return NSPredicate(format: "ANY searchResults.keyword = %@", keyword)


Yep! Don't forget both places, this is the one for the CustomerSearchResults rather than the Customer

joshheald · 2022-10-25T17:44:35Z

...erce/Classes/ViewRelated/Orders/Order Creation/CustomerSection/CustomerSearchUICommand.swift

+    var searchBarAccessibilityIdentifier: String = "customer-search-screen-search-field"
+
+    var cancelButtonAccessibilityIdentifier: String = "customer-search-screen-cancel-button"


You probably only need to set accessibilityIdentifiers if you're actually using the search in a UI test. Identifiers are nothing to do with VoiceOver and other assistive tech, in practice, they are only used for UI tests.

AccessibilityLabels are for VoiceOver and important to have on custom controls or complex groups.

joshheald · 2022-10-25T17:52:02Z

General observation: from my test site, the feature feels a bit... frustrating, in terms of the search results it can actually return. This is just a limitation of the API AFAIK, but I have lots of customer details on different orders, where the customers checked out as guests. In the Analytics section of wc-admin, it manages to link things together for those customers, even though they're guests... it certainly would be good if we could do a bit more like that for filling address details!

The brief here is to get to parity with Android, but if you do have thoughts or findings which could feed in to an iteration 2 of the feature, it would be good to hear them...

iamgabrielma · 2022-10-26T07:41:51Z

The search doesn't behave the same as on Android. On Android, search looks at the email field only, and on iOS, it looks at the name fields only. Apologies if I'm forgetting some context here, if Android are going to change that...

Yes, this was one of the API limitations when taking this approach (pe5pgL-3r-p2)

One big problem: search doesn’t search names, phones, and emails at the same time. You’ll have to use searchby and define what field you want to search.

I believe that, depending on when we plan to change the API behaviour, we could use this to our advantage and create some sort of "advanced search" with searchBy, which would also improve your point about the search feature being frustrating, which I agree with.

Add SwiftUI wrapper for SearchViewController

57e37ed

iamgabrielma added type: enhancement A request for an enhancement. category: parity Match what's supported by the other platform. feature: order creation All tasks related to creating an order labels Oct 19, 2022

iamgabrielma added this to the 10.9 milestone Oct 19, 2022

iamgabrielma mentioned this pull request Oct 19, 2022

Order creation: Search Customers #7741

Closed

15 tasks

iamgabrielma added 2 commits October 19, 2022 18:33

Make OrderCustomerListView its own file

7dcc1bc

Present OrderCustomerListView when tapped

ca8d350

iamgabrielma changed the base branch from trunk to issue/7741-add-customer-upsert-logic October 20, 2022 02:01

iamgabrielma added 4 commits October 20, 2022 11:32

Add CustomerSearchUICommand for Customer search

5b50032

Add localization enum & a11y strings

84eaab7

Sync search UI screen with searchCustomers action

6281456

Add firstName and lastName to Cell title

1605feb

iamgabrielma marked this pull request as ready for review October 20, 2022 03:20

Add callback to pass Customer data between views

7f42204

iamgabrielma commented Oct 20, 2022

View reviewed changes

iamgabrielma requested review from Ecarrion, ThomazFB and rachelmcr October 20, 2022 04:38

iamgabrielma mentioned this pull request Oct 20, 2022

Customer search: Fill customer fields on Order screen #7885

Merged

5 tasks

Base automatically changed from issue/7741-add-customer-upsert-logic to trunk October 21, 2022 15:16

spencertransier modified the milestones: 10.9, 11.0 Oct 22, 2022

iamgabrielma mentioned this pull request Oct 24, 2022

Add tracks for Customer Search #7907

Merged

3 tasks

iamgabrielma assigned joshheald and iamgabrielma Oct 25, 2022

iamgabrielma commented Oct 25, 2022

View reviewed changes

joshheald approved these changes Oct 25, 2022

View reviewed changes

iamgabrielma added 2 commits October 26, 2022 15:41

Add NSPredicate for siteID

a8dd778

Revert unintended XCRemoteSwiftPackageReference change

da5e401

iamgabrielma merged commit 0ba7063 into trunk Oct 26, 2022

iamgabrielma deleted the issue/741-implement-customer-search-ui branch October 26, 2022 07:45

iamgabrielma mentioned this pull request Oct 26, 2022

Customer Search: Empty state for non-results searches #7941

Closed

		var searchBarAccessibilityIdentifier: String = "customer-search-screen-search-field"

		var cancelButtonAccessibilityIdentifier: String = "customer-search-screen-cancel-button"

Customer Search: Implement UI #7880

Customer Search: Implement UI #7880

Uh oh!

Conversation

iamgabrielma commented Oct 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Testing instructions

Uh oh!

wpmobilebot commented Oct 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iamgabrielma Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamgabrielma commented Oct 25, 2022

Uh oh!

iamgabrielma Oct 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamgabrielma Oct 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshheald left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshheald commented Oct 25, 2022

Uh oh!

iamgabrielma commented Oct 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

iamgabrielma commented Oct 19, 2022 •

edited

Loading

wpmobilebot commented Oct 19, 2022 •

edited

Loading

iamgabrielma Oct 20, 2022 •

edited

Loading

iamgabrielma Oct 25, 2022 •

edited

Loading

iamgabrielma Oct 26, 2022 •

edited

Loading